-
Notifications
You must be signed in to change notification settings - Fork 254
Proc-macros allow default values without needing to specify a literal. #2543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This makes it possible to say you want the default value for a type rather than specifying a literal - eg, `#[uniffi(default)]` and similarly for defaults args for functions. Supports named types but it's up to the consumer to ensure they can be created without args. via and fixes mozilla#2538
Fixes #1653 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
@@ -20,6 +20,15 @@ | |||
|
|||
- HashMaps in UDL now support a default value with an empty map ([#2539](https://github.com/mozilla/uniffi-rs/pull/2539)). | |||
|
|||
XXX - thiss change log is missing 29.2?? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I forgot to open a PR after releasing 0.29.2. Here it is now though.: #2546
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice, just had some documentation suggestions. Feel free to take or leave the table markdown I wrote up.
- All builtin types have default values (zero, false, empty) | ||
- Named types (ie, Records, Objects, etc) are supported, but you must ensure the | ||
item is able to be created without arguments (eg, has a default constructor | ||
with no args, is a record with all fields having defaults, etc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks right to me, but I think describing 1 type per row would be clearer. It's less compact, but I think it's worth the space. Maybe a table would be better:
Type | Default value |
---|---|
Option |
None |
String | Empty string |
Vec<T> |
Empty list |
HashMap<T> |
Empty map |
Builtin numeric types | 0 |
bool |
false |
Records | Record with all fields set to their default value (only valid if they all have defaults) |
Objects | Primary constructor called with 0 arguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks - that just makes the section below this look even worse, but I just opened #2551 instead of getting distracted by that.
@jplatte @bendk like we spoke about, this seems quite nice to me. It's a breaking change (and I think the changelog is messed up, but we can sort that out)
This makes it possible to say you want the default value for a type rather than specifying a literal - eg,
#[uniffi(default)]
and similarly for defaults args for functions.Supports named types but it's up to the consumer to ensure they can be created without args.
via and fixes #2538